Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable accessing Message in TableView #1174

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

petermnhull
Copy link
Contributor

@petermnhull petermnhull commented Feb 10, 2024

Fixes #1176

Motivation

The current TableView implementation only stores the payload encoded to the SchemaValueType of each incoming message. There are many use cases where accessing the full metadata of the pulsar.Message type is needed, so this PR refactors the map storage of messages to be the pulsar.Message received from the consumer rather than the payload.

All the existing APIs are maintained by performing the payload encoding when the methods (e.g. Get) are called.

Two new public methods, Message and Messages, are introduced to access the pulsar.Message for each key so that accompanying metadata for a payload can be easily retrieved.

Modifications

  • The TableView implementation handleMessage method now stores the pulsar.Message type and only encodes to the SchemaValueType for listeners.
  • Get and Entries use the SchemaValueType when called to convert from pulsar.Message to the payload
  • Two public methods, Message and Messages, introduced to get full metadata.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests, and is covered by existing tests, and can be verified as follows:

  • Running table_view_test.go which runs new and existing tests for all functionality.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): No
  • The public API: Yes, new Message and Messages methods on TableView interface. Existing API not impacted
  • The schema: No
  • The default values of configurations: No
  • The wire protocol: No

Documentation

  • Does this pull request introduce a new feature? Yes
  • If yes, how is the feature documented? Code documentation, and happy to create new issue for other required documentation.

Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR invokes new API changes. We need to draft a PIP for it: https://github.com/apache/pulsar/tree/master/pip


func (tv *TableViewImpl) Messages() map[string]Message {
tv.dataMu.Lock()
defer tv.dataMu.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is returning the same underlying map, allowing for race-conditions and modifications from the application.

Note: the existing code is also wrong because it was returning tv.data instead of data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable accessing Message in TableView
3 participants